Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MWPW-169416 [MEP] stop treating remove action differently in preview #3789

Open
wants to merge 2 commits into
base: stage
Choose a base branch
from
Open

MWPW-169416 [MEP] stop treating remove action differently in preview #3789

wants to merge 2 commits into from

Conversation

vgoodric
Copy link
Contributor

@vgoodric vgoodric commented Mar 7, 2025 •
edited
Loading

  • No longer has a different behavior in preview when removing content or a fragment

Resolves: MWPW-169416

The goal of this PR is to remove the difference between preview and the MEP button being off (prod without MEP param or preview with mepButton=off param).

  1. In the before link, the first text block still takes up less of the page width because the section-metadata is still working. You can also see the section-metadata in the DOM. In the after and MEP button off links, the block is full width and the section-metadata block is not in the DOM.
  2. In the before link, highlight changes shows there is a deleted fragment and it is still in the DOM. In the after and MEP button off links it is fully removed.

Test URLs:

Marked high priority because it makes the QA for a test more difficult.

@aem-code-sync AEM Code Sync
Copy link
Contributor

aem-code-sync bot commented Mar 7, 2025 •
edited
Loading

Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
In case there are problems, just click a checkbox below to rerun the respective action.

  • Re-run PSI checks
  • Re-sync branch
Commits

@vgoodric vgoodric changed the title truly remove elements, even in preview MWPW-169416 [MEP] stop treating remove action differently in preview Mar 7, 2025
@aem-code-sync AEM Code Sync
Copy link
Contributor

aem-code-sync bot commented Mar 7, 2025 •
edited
Loading

Page Scores Audits Google
📱 /?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
🖥️ /?martech=off PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@vgoodric vgoodric added the high priority Why is this a high priority? Blocker? Critical? Dependency? label Mar 7, 2025
@vgoodric vgoodric requested a review from a team March 7, 2025 21:20
markpadbe
markpadbe approved these changes Mar 7, 2025
View reviewed changes
Copy link
Contributor

@markpadbe markpadbe left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid confusion, would it be worth adding some text to the Highlight changes checkbox text. For example:

Screenshot 2025-03-07 at 2 07 19 PM

Otherwise, the change looks good to me.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Mar 8, 2025

This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markpadbe markpadbe markpadbe approved these changes

@ivanvatadobe ivanvatadobe ivanvatadobe approved these changes

Assignees
No one assigned
Labels
high priority Why is this a high priority? Blocker? Critical? Dependency?
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@vgoodric @markpadbe @ivanvatadobe